Skip to content

Add mountNode prop to Popup module - #4362

Open
svicalifornia wants to merge 1 commit into
Semantic-Org:masterfrom
attivonetworks:v2.1.2-popup-mount-node
Open

Add mountNode prop to Popup module#4362
svicalifornia wants to merge 1 commit into
Semantic-Org:masterfrom
attivonetworks:v2.1.2-popup-mount-node

Conversation

@svicalifornia

Copy link
Copy Markdown

If a React app with SUIR is running in a hidden IFRAME and rendering some or all of its virtual DOM to the parent frame or another frame, then the Popup component should not insert popup elements into the document.body of the React IFRAME, but instead should inject into the frame containing the Popup trigger, or even into some other container as may be more appropriate (due to size or layering concerns between multiple frames).

Adding a mountNode prop to Popup allows developers to specify the appropriate container for a displayed popup element in cases such as the above example.

This PR implements a mountNode prop for Popup, with code very similar to the same prop's implementation in Modal.

If a React app with SUIR is running in a hidden IFRAME and rendering
some or all of its virtual DOM to the parent frame or another frame,
then the Popup component should not insert popup elements into the
document.body of the React IFRAME, but instead should inject into the
frame containing the Popup trigger, or even into some other container
as may be more appropriate (due to size or layering concerns between
multiple frames).

Adding a `mountNode` prop to Popup allows developers to specify the
appropriate container for a displayed popup element in cases such as
the above example.
@welcome

welcome Bot commented Apr 29, 2022

Copy link
Copy Markdown

💖 Thanks for opening this pull request! 💖

Here is a list of things that will help get it across the finish line:

  • Run yarn lint locally to catch formatting errors. This will fix some errors automatically, commit and push any changes.
  • Run yarn test locally to catch errors. This ensures all components still behave as they should.
  • Run yarn start to run the doc site locally and try a few pages, ensuring everything is in good working order.
  • Include tests when adding/changing behavior.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@codecov-commenter

codecov-commenter commented Apr 29, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.75%. Comparing base (49f0056) to head (7832ab6).
⚠️ Report is 40 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4362   +/-   ##
=======================================
  Coverage   99.75%   99.75%           
=======================================
  Files         180      180           
  Lines        3248     3250    +2     
=======================================
+ Hits         3240     3242    +2     
  Misses          8        8           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants